-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
explicitly fail when ssl setup fails in socket_init #4402
base: devel
Are you sure you want to change the base?
Conversation
Currently in socket_init(), it doesn't check the ret code on ssl_setup_connection_params() call. the initialization appears successful but the transport is not usable, ssl handshake all fails. There are many failure scenarios like cert/key/ca file absence, or cert/key mismatch, that cause the ssl setup failing. It's better to fail the initialization explicitly so that we can discover the problem much earlier. There are so many Signed-off-by: Jiankun Yu <[email protected]>
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
CLANG-FORMAT FAILURE: index d43589ea6..97599e24b 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -56,7 +56,8 @@
#if !defined(DEFAULT_VERIFY_DEPTH)
#define DEFAULT_VERIFY_DEPTH 1
#endif
-#define DEFAULT_CIPHER_LIST "AES128:EECDH:EDH:HIGH:!3DES:!RC4:!DES:!MD5:!aNULL:!eNULL"
+#define DEFAULT_CIPHER_LIST \
+ "AES128:EECDH:EDH:HIGH:!3DES:!RC4:!DES:!MD5:!aNULL:!eNULL"
#define DEFAULT_DH_PARAM SSL_CERT_PATH "/dhparam.pem"
#define DEFAULT_EC_CURVE "prime256v1"
|
/run regression |
Hi, @mohit84 Some of the regression tests are failing and I didn't figure out how they are triggered on github. Could you enlighten me about the github checks and tests? I might submit PRs in the future for minor improvements/bugfixes. |
The failed test cases are not mandatory so we can ignore them. |
recheck smoke |
/recheck smoke |
CLANG-FORMAT FAILURE: index d43589ea6..97599e24b 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -56,7 +56,8 @@
#if !defined(DEFAULT_VERIFY_DEPTH)
#define DEFAULT_VERIFY_DEPTH 1
#endif
-#define DEFAULT_CIPHER_LIST "AES128:EECDH:EDH:HIGH:!3DES:!RC4:!DES:!MD5:!aNULL:!eNULL"
+#define DEFAULT_CIPHER_LIST \
+ "AES128:EECDH:EDH:HIGH:!3DES:!RC4:!DES:!MD5:!aNULL:!eNULL"
#define DEFAULT_DH_PARAM SSL_CERT_PATH "/dhparam.pem"
#define DEFAULT_EC_CURVE "prime256v1"
|
Currently in socket_init(), it doesn't check the ret code on ssl_setup_connection_params() call. the initialization appears successful but the transport is not usable, ssl handshake all fails.
There are many failure scenarios like cert/key/ca file absence, or cert/key mismatch, that cause the ssl setup failing.
It's better to fail the initialization explicitly so that we can discover the problem much earlier. There are so many